-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[NV TensorRT RTX EP] Fix external tensorrt_plugins load path #26814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NV TensorRT RTX EP] Fix external tensorrt_plugins load path #26814
Conversation
|
@devang-ml , @chilo-ms , can you please review and merge this PR? It is required for current EP drop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the external tensorrt_plugins library loading path to use the EP (Execution Provider) library location instead of the runtime library location. This addresses an issue where the plugin library was being incorrectly searched for in the core ONNX Runtime library path, which is separate from the EP library path in WinML workflows.
Key changes:
- Introduced a new
GetEPLibraryDirectory()helper function to determine the EP library's directory path - Updated the plugin loading logic to use the EP library directory instead of the runtime path
- Updated the error log message to reflect the correct search location
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (last_slash != std::string::npos) { | ||
| return PathString(so_path.substr(0, last_slash + 1)); | ||
| } | ||
| return PathString(so_path); |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On line 70, when last_slash is not found (npos), the function returns the entire path string. This would be incorrect because if there's no slash in the path, the function should return an empty string instead of the full path, since the purpose is to return the directory containing the library, not the library path itself. Consider returning an empty PathString when last_slash == std::string::npos.
| // Get handle to the DLL executing this code | ||
| if (!GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | | ||
| GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, | ||
| reinterpret_cast<LPCWSTR>(&GetEPLibraryDirectory), |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reinterpret_cast to LPCWSTR is incorrect. GetModuleHandleExW expects the lpModuleName parameter to be a string (LPCWSTR) when GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS is not used, but when this flag is set, it expects a raw address (LPCVOID). The address of the function should be cast to LPCWSTR without going through the pointer, i.e., reinterpret_cast(GetEPLibraryDirectory) should be used, not reinterpret_cast(&GetEPLibraryDirectory).
| if (lastSlash != std::wstring::npos) { | ||
| return PathString(path.substr(0, lastSlash + 1)); | ||
| } | ||
| return PathString(path); |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On line 56, when lastSlash is not found (npos), the function returns the entire path string. This would be incorrect because if there's no slash in the path, the function should return an empty string instead of the full path, since the purpose is to return the directory containing the library, not the library path itself. Consider returning an empty PathString when lastSlash == std::wstring::npos.
onnxruntime/core/providers/nv_tensorrt_rtx/nv_execution_provider_custom_ops.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/nv_tensorrt_rtx/nv_execution_provider_custom_ops.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/nv_tensorrt_rtx/nv_execution_provider_custom_ops.cc
Outdated
Show resolved
Hide resolved
yuslepukhin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🕐
|
Please, comment and resolve Copilot issues. |
yuslepukhin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
|
/azp run Linux QNN CI Pipeline, Win_TRT_Minimal_CUDA_Test_CI,Windows ARM64 QNN CI Pipeline,Windows GPU Doc Gen CI Pipeline,Windows x64 QNN CI Pipeline |
|
Azure Pipelines successfully started running 4 pipeline(s). |
…ft#26814) ### Description <!-- Describe your changes. --> Load tensorrt_plugin library from EP library location instead of runtime library location. ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. --> Plugin library was being searched for in core ONNX Runtime library path, and not the EP library path. These paths are separate in case of WinML workflow.
### Description <!-- Describe your changes. --> Load tensorrt_plugin library from EP library location instead of runtime library location. ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. --> Plugin library was being searched for in core ONNX Runtime library path, and not the EP library path. These paths are separate in case of WinML workflow.
| Commit | Commit Title | Author | | :--- | :--- | :--- | | `11dde2d9e` | [NV TensorRT RTX EP] Fix external tensorrt_plugins load path (#26814) | keshavv27 | | `080d96818` | Move model compatibility checks ahead of session initialization (#27037) | adrastogi | | `ec4f6bfa1` | [QNN EP] Fix error messages being logged as VERBOSE instead of ERROR (#24931) | Copilot | | `0432e7125` | perftest: support plugin eps for compile_ep_context (#27121) | Jaskaran Singh Nagi | | `727db0d3d` | Engine compatibility validity API implementation (#26774) | umangb-09 | | `27013522f` | Deprecate transformers model examples (#27156) | Jambay Kinley | | `f83d4d06e` | [QNN-EP] Implement file mapped weights feature (#26952) | quic-calvnguy | --------- Co-authored-by: keshavv27 <[email protected]> Co-authored-by: adrastogi <[email protected]> Co-authored-by: Aditya Rastogi <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: vraspar <[email protected]> Co-authored-by: yuslepukhin <[email protected]> Co-authored-by: Dmitri Smirnov <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Dmitri Smirnov <[email protected]> Co-authored-by: Jaskaran Singh Nagi <[email protected]> Co-authored-by: umangb-09 <[email protected]> Co-authored-by: Jambay Kinley <[email protected]> Co-authored-by: quic-calvnguy <[email protected]> Co-authored-by: quic_calvnguy <quic_calvnguy@quic_inc.com>
Description
Load tensorrt_plugin library from EP library location instead of runtime library location.
Motivation and Context
Plugin library was being searched for in core ONNX Runtime library path, and not the EP library path. These paths are separate in case of WinML workflow.